From df222a5a96cb92c518428474b03dbe8c05af6bd2 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Sat, 13 Jun 2015 16:59:35 +0300 Subject: [PATCH] babl_format_new(): fix global-buffer-overflow If we pass a string into this function, and this string is shorter than sizeof(Babl), macro BABL_IS_BABL() will read past string bounds, and bad things may happen. NOTE: if a string will be passed into this function, that is not handled by those if (!strcmp (arg, "<...>")), global-buffer-overflow will still happen. i am not sure if/what can be done about it :( Fixes following error: ================================================================= ==1657==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fd3026d6c20 at pc 0x7fd3026b6e4a bp 0x7fffaac308a0 sp 0x7fffaac30898 READ of size 4 at 0x7fd3026d6c20 thread T0 0 0x7fd3026b6e49 in babl_format_new /home/lebedevri/src/_GIMP/babl/babl/babl-format.c:317 1 0x7fd3026bf44f in construct_double_format /home/lebedevri/src/_GIMP/babl/babl/babl-model.c:259 2 0x7fd3026bfd12 in babl_model_new /home/lebedevri/src/_GIMP/babl/babl/babl-model.c:204 3 0x7fd3026acce5 in babl_core_init /home/lebedevri/src/_GIMP/babl/babl/babl-core.c:128 4 0x7fd3026a9379 in babl_init /home/lebedevri/src/_GIMP/babl/babl/babl.c:145 5 0x7fd306e5a3d1 in gegl_post_parse_hook (/usr/local/lib/libgegl-0.3.so.0+0x523d1) 6 0x7fd301f5d238 in g_option_context_parse (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x55238) 7 0x7fd301f5e193 in g_option_context_parse_strv (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x56193) 8 0x48b8cf in main (/usr/local/bin/gimp-2.9+0x48b8cf) 9 0x7fd300f71b44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44) 10 0x486b68 (/usr/local/bin/gimp-2.9+0x486b68) 0x7fd3026d6c23 is located 0 bytes to the right of global variable '*.LC8' from 'babl-model.c' (0x7fd3026d6c20) of size 3 '*.LC8' is ascii string 'id' SUMMARY: AddressSanitizer: global-buffer-overflow /home/lebedevri/src/_GIMP/babl/babl/babl-format.c:317 babl_format_new Shadow bytes around the buggy address: 0x0ffae04d2d70: f9 f9 f9 f9 00 00 04 f9 f9 f9 f9 f9 00 00 02 f9 =>0x0ffae04d2d80: f9 f9 f9 f9[03]f9 f9 f9 f9 f9 f9 f9 00 02 f9 f9 0x0ffae04d2d90: f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Contiguous container OOB:fc ASan internal: fe ==1657==ABORTING --- babl/babl-format.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/babl/babl-format.c b/babl/babl-format.c index 757ae58..60a916e 100644 --- a/babl/babl-format.c +++ b/babl/babl-format.c @@ -314,7 +314,29 @@ babl_format_new (const void *first_arg, while (1) { - if (BABL_IS_BABL (arg)) + /* first, we assume arguments to be strings */ + if (!strcmp (arg, "id")) + { + id = va_arg (varg, int); + } + + else if (!strcmp (arg, "name")) + { + name = babl_strdup (va_arg (varg, char *)); + } + + else if (!strcmp (arg, "packed")) + { + planar = 0; + } + + else if (!strcmp (arg, "planar")) + { + planar = 1; + } + + /* if we didn't point to a known string, we assume argument to be babl */ + else if (BABL_IS_BABL (arg)) { Babl *babl = (Babl *) arg; @@ -377,26 +399,6 @@ babl_format_new (const void *first_arg, break; } } - /* if we didn't point to a babl, we assume arguments to be strings */ - else if (!strcmp (arg, "id")) - { - id = va_arg (varg, int); - } - - else if (!strcmp (arg, "name")) - { - name = babl_strdup (va_arg (varg, char *)); - } - - else if (!strcmp (arg, "packed")) - { - planar = 0; - } - - else if (!strcmp (arg, "planar")) - { - planar = 1; - } else { -- 2.30.2